Skip to content

fix(vad): guard unknown field limits and normalize context in whisperVadConfig - #1926

Closed
hsusul wants to merge 1 commit into
OpenWhispr:mainfrom
hsusul:fix/1925-whisper-vad-config-safety
Closed

fix(vad): guard unknown field limits and normalize context in whisperVadConfig#1926
hsusul wants to merge 1 commit into
OpenWhispr:mainfrom
hsusul:fix/1925-whisper-vad-config-safety

Conversation

@hsusul

@hsusul hsusul commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Fixes #1925

Problem

In src/helpers/whisperVadConfig.js:

  • clampVadField(key, value) unconditionally indexed LIMITS[key], throwing TypeError: Cannot destructure property 'min' of 'LIMITS[key]' if an unlisted or custom configuration field was passed.
  • resolveContextSileroEnabled(settings, context) compared against literal "dictation", "noteRecording", and "meeting", failing to recognize mixed-case or whitespace-padded context strings.

Solution

  • Checked for existence of LIMITS[key] before destructuring min/max limits.
  • Normalized context string with .trim().toLowerCase().
  • Added unit tests in test/helpers/whisperVadConfig.test.js.

Verification

  • node --test test/helpers/whisperVadConfig.test.js (passes, 5/5 tests)
  • npm run typecheck (passes, 0 errors)
  • npm run lint (passes, 0 errors)
  • npm run i18n:check (passes)
  • npm run build:renderer (passes)
  • git diff --check (clean)

@gabrielste1n

Copy link
Copy Markdown
Collaborator

Thanks for the contribution, @hsusul! Closing after tracing this end to end: clampVadField has exactly one production call site — sanitizeWhisperVadConfig (src/helpers/whisperVadConfig.js:21) — which iterates Object.keys(DEFAULTS), and the DEFAULTS and LIMITS key sets in src/constants/whisperVad.json are identical, so LIMITS[key] can never be undefined; the renderer-facing whisper-vad-set-config IPC payload is additionally whitelisted in _setWhisperVadSettings (ipcHandlers.js:697) before it can touch this path. resolveContextSileroEnabled likewise has exactly one production call site (ipcHandlers.js:721 via _resolveWhisperVadOptions), whose five callers all pass the hardcoded literals "dictation" (ipcHandlers.js:2880, 7509), "noteRecording" (2693, 5822), and "meeting" (7214) — the context string is never renderer-, file-, or network-supplied, so mixed-case or padded values (and the new "note_recording" alias, which appears nowhere in the codebase) never occur. Per our convention (validate once at real trust boundaries, trust internal callers past that point), these guards would only convert a future caller bug into a silently unclamped passthrough instead of a loud failure. Closing #1925 along with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

clampVadField throws TypeError on unknown keys and resolveContextSileroEnabled misses mixed-case contexts

2 participants